Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add program property and PLT for CFI extension #417

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

kito-cheng
Copy link
Collaborator

We introduce .note.gnu.property section to store infomations that linker or runtime system may use, and we define two bit for landing pad and shadow stack.

Those two bit will checked by loader - and use that info to enable the landing pad checking and/or shadow stack feature.

@kito-cheng
Copy link
Collaborator Author

cc @ved-rivos

@rui314
Copy link
Collaborator

rui314 commented Jan 5, 2024

My concern about this proposal is that, with the 32-bit bitmap, we can't have more than 32 extensions. That might be OK for x86-64 that is controlled only by Intel and AMD, but it is likely that we would have way more extensions than x86-64 for RISC-V. I'm not sure even 64 bits is enough. Maybe we should avoid the bitmap completely?

@kito-cheng
Copy link
Collaborator Author

@rui314
GNU property is kind of key-value structure, key is 32 bit integer, and we have 536,870,912 can use for the key per the definition of GNU_PROPERTY_LOPROC and GNU_PROPERTY_HIPROC.

/* Processor-specific semantics, lo */
#define GNU_PROPERTY_LOPROC                     0xc0000000
/* Processor-specific semantics, hi */
#define GNU_PROPERTY_HIPROC                     0xdfffffff

So we have 536,870,912 * 32 rather than only 32-bit bitmap :)

@rui314
Copy link
Collaborator

rui314 commented Jan 5, 2024

@kito-cheng Ah, you are right. Even though I've implemented the feature to the linker, I forgot the actual on-file format. Thank you for pointing that out!

@MaskRay
Copy link
Collaborator

MaskRay commented Jan 8, 2024

Thanks for the proposal. On Arm/x86, if all relocatable files contain .note.gnu.property (which contains NT_GNU_PROPERTY_TYPE_0 notes) with the GNU_PROPERTY_X86_FEATURE_1_IBT bit set, the linker will mark the output as compatible with the feature. Assembly files often lack .note.gnu.property, making the CFI features difficult to deploy...

That said, this is most viable proposal. Defining GNU program property looks good for x86/Arm parity.

You may want to mention that n_type is NT_GNU_PROPERTY_TYPE_0.

Add property for CFI extension

"property" is a heavily overloaded term. Consider "program property" or just use .note.gnu.property to be more specific.

@sorear
Copy link
Collaborator

sorear commented Feb 18, 2024

@kito-cheng Isn't the usefulness of this degraded by the fact that we only allocate 1 32-bit property instead of the ~ 3 * 32768 allocated on x86? When we start to use the next bitmap, an old linker won't be able to propagate it, and so on.

kito-cheng added a commit that referenced this pull request Feb 26, 2024
We introduce .note.gnu.property section to store infomations that linker
or runtime system may use, and we define 4 program property classes to
specifying the merge semantics, it's used for forward compatibility in linker
implementations, allowing linker can correctly handle program property types
even if they are not recognized.

We don't define any program property within this PR, it would be
separate PR like #417
@kito-cheng kito-cheng mentioned this pull request Feb 26, 2024
@kito-cheng
Copy link
Collaborator Author

Split base program property part into #428

kito-cheng added a commit that referenced this pull request Feb 26, 2024
We introduce .note.gnu.property section to store infomations that linker
or runtime system may use, and we define 4 program property classes to
specifying the merge semantics, it's used for forward compatibility in linker
implementations, allowing linker can correctly handle program property types
even if they are not recognized.

We don't define any program property within this PR, it would be
separate PR like #417
@kito-cheng kito-cheng changed the base branch from master to prog-prop February 26, 2024 09:56
@kito-cheng
Copy link
Collaborator Author

ChangeLog:

  • Based on Add program property #428
  • Rename GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP to GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE
  • Rename GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS to GNU_PROPERTY_RISCV_FEATURE_1_CFI_SS
  • Explicitly specify the labeling scheme in GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE, label will always set to 1.

@kito-cheng kito-cheng changed the title Add program property for CFI extension Add program property and PLT for CFI extension Feb 26, 2024
@kito-cheng
Copy link
Collaborator Author

Changes:

  • Explicitly specify the labeling scheme in GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE, label will always set to 0.
  • Add simple landing pad PLT.
  • Add DT_RISCV_SIMPLE_LP_PLT to mark this object use simple landing pad PLT.

kito-cheng added a commit that referenced this pull request Jul 16, 2024
We introduce .note.gnu.property section to store infomations that linker
or runtime system may use, and we define 4 program property classes to
specifying the merge semantics, it's used for forward compatibility in linker
implementations, allowing linker can correctly handle program property types
even if they are not recognized.

We don't define any program property within this PR, it would be
separate PR like #417
kito-cheng added a commit that referenced this pull request Jul 16, 2024
We introduce .note.gnu.property section to store infomations that linker
or runtime system may use, and we define 4 program property classes to
specifying the merge semantics, it's used for forward compatibility in linker
implementations, allowing linker can correctly handle program property types
even if they are not recognized.

We don't define any program property within this PR, it would be
separate PR like #417
Define two bit for landing pad and shadow stack, and we plan to defined
third bit `GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_COMPLEX` for complex
labeling scheme.
Changes:
- Rename `GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE` to `GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_UNLABELED`
- Fix wrong offset in the first PLT stubs for the simple landing pad PLT.
@kito-cheng
Copy link
Collaborator Author

Changes:

  • Rename GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE to GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_UNLABELED
  • Fix wrong offset in the first PLT stubs for the simple landing pad PLT.

yetingk pushed a commit to sifive/riscv-llvm that referenced this pull request Jul 18, 2024
This patch implements simple landing pad labels [0]. When Zicfilp enabled, this
patch inserts `lpad 0` at the beginning of basic blocks which are possible to be
landed by indirect jumps.
This patch also supports option riscv-landing-pad-label to make users
cpable to set nonzero fixed labels. Using nonzero fixed label force
setting t2 before indirect jumps. It's less portable but more strict than
original implementation.

[0]: riscv-non-isa/riscv-elf-psabi-doc#417
yetingk pushed a commit to sifive/riscv-llvm that referenced this pull request Jul 18, 2024
This is cherry-picked from upstream pr #91860.

This patch is based on #91855. This patch inserts simple landing pad
([pr])before indirct jumps. And also make option riscv-landing-pad-label
influence this feature.
[pr]: riscv-non-isa/riscv-elf-psabi-doc#417
Base automatically changed from prog-prop to master August 2, 2024 06:48
yetingk pushed a commit to yetingk/llvm-project that referenced this pull request Aug 5, 2024
This patch implements simple landing pad labels [0]. When Zicfilp enabled, this
patch inserts `lpad 0` at the beginning of basic blocks which are possible to be
landed by indirect jumps.
This patch also supports option riscv-landing-pad-label to make users
cpable to set nonzero fixed labels. Using nonzero fixed label force
setting t2 before indirect jumps. It's less portable but more strict than
original implementation.

[0]: riscv-non-isa/riscv-elf-psabi-doc#417
yetingk pushed a commit to yetingk/llvm-project that referenced this pull request Aug 5, 2024
This patch is based on llvm#91855. This patch inserts simple landing pad
([pr])before indirct jumps. And also make option riscv-landing-pad-label
influence this feature.
[pr]: riscv-non-isa/riscv-elf-psabi-doc#417
yetingk added a commit to llvm/llvm-project that referenced this pull request Aug 6, 2024
This patch implements simple landing pad labels ([pr]). When Zicfilp
enabled, this patch inserts `lpad 0` at the beginning of basic blocks
which are possible to be landed by indirect jumps.
This patch also supports option riscv-landing-pad-label to make users
cpable to set nonzero fixed labels. Using nonzero fixed label force
setting t2 before indirect jumps. It's less portable but more strict
than original implementation.

[pr]: riscv-non-isa/riscv-elf-psabi-doc#417
yetingk pushed a commit to yetingk/llvm-project that referenced this pull request Aug 6, 2024
This patch is based on llvm#91855. This patch inserts simple landing pad
([pr])before indirct jumps. And also make option riscv-landing-pad-label
influence this feature.
[pr]: riscv-non-isa/riscv-elf-psabi-doc#417
yetingk added a commit to llvm/llvm-project that referenced this pull request Aug 8, 2024
…91860)

This patch is based on #91855.
This patch inserts simple landing pad
([pr])before indirct jumps. And this also make option
riscv-landing-pad-label influence this feature.
[pr]: riscv-non-isa/riscv-elf-psabi-doc#417
@kito-cheng
Copy link
Collaborator Author

We have implementation on both toolchain, so I think we are ready to land for this PR :)

Let me know if further comment on this

Copy link

@mylai-mtk mylai-mtk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kito-cheng It looks like the content of this PR is stable and ready to be merged, so I guess it the time to review it once again to reconfirm my knowledge and fine tune for wording, garmmar, etc. I've found quite a few typing mistakes/inappropriates in the process, but the content is good. Thanks for the effort!

@@ -643,15 +643,29 @@ The PLT (Procedure Linkage Table) exists to allow function calls between
dynamically linked shared objects. Each dynamic object has its own
GOT (Global Offset Table) and PLT (Procedure Linkage Table).

RISC-V has defined several PLT styles, which used for different situation,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: RISC-V defines several PLT styles, which are used in different situations.

@@ -643,15 +643,29 @@ The PLT (Procedure Linkage Table) exists to allow function calls between
dynamically linked shared objects. Each dynamic object has its own
GOT (Global Offset Table) and PLT (Procedure Linkage Table).

RISC-V has defined several PLT styles, which used for different situation,
the default PLT sytle should be used if the program is not met the condition for

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: The default PLT style should be used if the program does not meet the condition conditions for

@@ -643,15 +643,29 @@ The PLT (Procedure Linkage Table) exists to allow function calls between
dynamically linked shared objects. Each dynamic object has its own
GOT (Global Offset Table) and PLT (Procedure Linkage Table).

RISC-V has defined several PLT styles, which used for different situation,
the default PLT sytle should be used if the program is not met the condition for
using all other PLT sytle.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: using all other PLT styles.

function pointer from the GOT. On the first call to a function, the
entry redirects to the first PLT entry which calls `_dl_runtime_resolve`
and fills in the GOT entry for subsequent calls to the function:
And occupies three 16 byte entries for the simple landing pad PLT style:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type: And occupies three 16 byte entries for the unlabeled landing pad PLT style:

Comment on lines +685 to +686
1: lpad 0
auipc t2, %pcrel_hi(.got.plt)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo:

    lpad 0
1:  auipc  t2, %pcrel_hi(.got.plt)

@@ -678,6 +710,15 @@ and fills in the GOT entry for subsequent calls to the function:
nop
----

The code sequences of the PLT entry for the the simple landing pad PLT style:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: The code sequences of the PLT entry for the the unlabeled landing pad PLT style:

Comment on lines +1442 to +1446
`GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_UNLABELED` This bit indicate that all
executable sections are built to be compatible with the landing pad mechanism
provided by the `Zicfilp` extension. An executable or shared library with this
bit set is required to generate PLTs with the landing pad (`lpad`) instruction,
and all label are set to `0`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrase to avoid confusing with the to-be-added func-sig scheme-specifics:

This bit indicates that all executable sections are built to be compatible with the landing pad mechanism
provided by the Zicfilp extension in the unlabeled scheme: Executables and shared libraries with this bit set are required to generate PLTs in the unlabeled landing pad PLT style, and all of the labels of lpad instructions are set to 0, i.e. unlabeled.

bit set is required to generate PLTs with the landing pad (`lpad`) instruction,
and all label are set to `0`.

`GNU_PROPERTY_RISCV_FEATURE_1_CFI_SS`: This bit indicate that all executable

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: GNU_PROPERTY_RISCV_FEATURE_1_CFI_SS: This bit indicates that all executable

the `Zicfiss` extension. Loading an executable or shared library with this bit
set requires the execution environment to provide either the `Zicfiss` extension
or the `Zimop` extension. When the executable or shared library is compiled with
compressed instructions then loading an executable with this bit set requires

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrase: compressed instructions then loading an executable it with this bit set requires

set requires the execution environment to provide either the `Zicfiss` extension
or the `Zimop` extension. When the executable or shared library is compiled with
compressed instructions then loading an executable with this bit set requires
the execution environment to provide the `Zicfiss` extension or to provide both

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrase: the execution environment to provide the Zicfiss extension or to provide both

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants